Clean-up classes nested in functions by ilevkivskyi · Pull Request #21478 · python/mypy

ilevkivskyi · 2026-05-13T12:32:33Z

TBH the current situation is embarrassing. I decided to finally clean this all up and unify various cases. Now we have same simple rules used by all classes, both regular (first three) and magic:

Use the name mangling for everything inside a top-level function.
Do not mangle defn.name only defn.fullname.
Always store classes nested in functions in global symbol table (using enclosing class was only adding unnecessary complications)
In cases of name mismatch (like One = NamedTuple("Other", ...) etc) always use the var_name for all purposes.
In cases of inline base classes use different mangling mechanism (since those may be nested in functions as well).

ilevkivskyi · 2026-05-14T00:07:40Z

It looks like this also fixes #13024 I am going to add a test case.

ilevkivskyi · 2026-05-14T00:17:53Z

Btw mypy_primer is also nice, users don't need to see those @252 suffixes in error messages, unless needed (i.e. unless format_type_distinctly will decide to use full names).

JukkaL

Thanks for cleaning this up! Looks good overall, but I have a few concerns about cases where we no longer include a line number.

JukkaL · 2026-05-22T10:17:01Z

+def f() -> None:
+    class X:
+        ...
+    x  # E: Name "x" is not defined


Lower case vs upper case X. If intended, maybe add comment, since they look pretty similar.

I am going to rename x to undefined.

JukkaL · 2026-05-22T10:36:48Z

    A
    TupleType(
-      tuple[Any, fallback=__main__.N@2])
+      tuple[Any, fallback=__main__.namedtuple@N])


Since we no longer have a line number, what happens if we have multiple namedtuple('N', ...') base classes in a single file?

I was going to say that this can't happen because there can't be two classes with the same name, but apparently we use the string name (i.e. N, not A) in such cases. I think a proper solution would be to use something like A@base1, etc.

To be clear I don't want to use line number for this disambiguation as well, since we can get weird things like Foo@7@7 if we need to apply both (e.g. namedtuple base class for a class nested in function).

I think a proper solution would be to use something like A@base1, etc.

Yes, this allowed to delete another couple dozen lines of tricky logic. I also added couple incremental tests to make sure all is good in such cases.

JukkaL · 2026-05-22T10:40:01Z

+        #     The second one is needed to properly serialize any classes nested in functions.
+        # TODO: make sure the daemon works well with these rules.
+        if self.is_nested_within_func_scope():
+            class_def.fullname = f"{self.cur_mod_id}.{name}@{line}"


This name generation logic is duplicated in a few places -- maybe move it to a helper function?

JukkaL · 2026-05-22T10:55:33Z

-            name += "@" + str(call.line)
+            #   * This is a local (function or method level) named tuple, this case is
+            #     handled by basic_new_typeinfo().
+            name = "namedtuple@" + name


As discussed in my other comment, I wonder if preserving the line number would be helpful here.

JukkaL · 2026-05-22T10:56:45Z

+
+            if var_name is None:
+                # Give it unique name in case a TypedDict() call is used as a base class.
+                name = "TypedDict@" + name


This looks similar to namedtuple case -- would the line number be helpful here, in case there are multiple base classes in the same file?

…lasses

github-actions · 2026-05-22T14:10:20Z

Diff from mypy_primer, showing the effect of this PR on open source code:

zulip (https://github.com/zulip/zulip)
- zerver/lib/user_topics.py:265: error: Argument 1 to "map" has incompatible type "Callable[[RecipientTopicDict@252], Any]"; expected "Callable[[dict[str, Any]], Any]"  [arg-type]
+ zerver/lib/user_topics.py:265: error: Argument 1 to "map" has incompatible type "Callable[[RecipientTopicDict], Any]"; expected "Callable[[dict[str, Any]], Any]"  [arg-type]
- zerver/lib/user_topics.py:303: error: Argument 1 to "map" has incompatible type "Callable[[RecipientTopicDict@252], Any]"; expected "Callable[[dict[str, Any]], Any]"  [arg-type]
+ zerver/lib/user_topics.py:303: error: Argument 1 to "map" has incompatible type "Callable[[RecipientTopicDict], Any]"; expected "Callable[[dict[str, Any]], Any]"  [arg-type]

JukkaL · 2026-05-22T15:00:06Z

+[file lib.py]
+from typing import NamedTuple
+class A(NamedTuple("N", [('x', int)])): pass
+class A(NamedTuple("N", [('x', str)])): pass


What if the second one is like class B(NamedTuple("N", ...)), i.e. only the base class string literal name is common?

As I said, string name is always ignored now. But I can add a test if you want.

This is fine.

JukkaL

Thanks for the updates!

JukkaL · 2026-05-22T16:52:23Z

+[file lib.py]
+from typing import NamedTuple
+class A(NamedTuple("N", [('x', int)])): pass
+class A(NamedTuple("N", [('x', str)])): pass


This is fine.

Clean-up classes nested in functions

a1e2963

ilevkivskyi requested review from JukkaL and hauntsaninja May 13, 2026 12:33

This comment has been minimized.

Sign in to view

ilevkivskyi mentioned this pull request May 13, 2026

Mypy plugin workaround for dynamic models defined in functions pydantic/pydantic#13177

Merged

5 tasks

Fix docstring

9eb18d3

This comment has been minimized.

Sign in to view

Add regression test

d732443

This comment has been minimized.

Sign in to view

JukkaL reviewed May 22, 2026

View reviewed changes

ilevkivskyi added 2 commits May 22, 2026 12:52

Merge remote-tracking branch 'upstream/master' into refactor-nested-c…

087d8dc

…lasses

Address CR

d603b42

This comment has been minimized.

Sign in to view

Avoid extra errors in edge cases; add tests

c6eff9d

JukkaL reviewed May 22, 2026

View reviewed changes

JukkaL approved these changes May 22, 2026

View reviewed changes

Uh oh!

Conversation

ilevkivskyi commented May 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment has been minimized.

This comment has been minimized.

ilevkivskyi commented May 14, 2026

Uh oh!

ilevkivskyi commented May 14, 2026

Uh oh!

This comment has been minimized.

JukkaL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

github-actions Bot commented May 22, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JukkaL left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ilevkivskyi commented May 13, 2026 •

edited

Loading